Skip to content

Conversation

@BrainforgeUK
Copy link
Contributor

@BrainforgeUK BrainforgeUK commented Sep 9, 2025

Pull Request for Issue # .

Summary of Changes

Added event handler response checks for:

  • onInstallerBeforePackageDownload
  • onInstallerBeforeUpdateSiteDownload

Added events:

  • AfterPackageDownloadEvent
  • AfterPackageDownloadFailedEvent

Testing Instructions

  1. Install these extensions - no need to enable them.
    plg_test_test1v1.zip
    plg_test_test2v1.zip

  2. Go to check for updates.
    v2.0.0 of these will be available.
    plg_test_test1
    plg_test_test2

  3. Update plg_test_test1
    Works fine, updated to v2.0.0

  4. Update plg_test_test2

  • Before this pull request .. 403 response error message.
  • After this pull request ... get subscription message from the update server.

Actual result BEFORE applying this Pull Request

error0-screenshot

Expected result AFTER applying this Pull Request

screenshot1

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

https://manual.joomla.org/docs/building-extensions/install-update/update-server/ will need to be updated along the following lines:

  • Update servers can now return JSON containing a message and other information.
    For instance the above test will return this (after PHP json_decode()):
    array (
    'message' => '<strong>plg_test_test2: </strong>The subscription key you provided is expired or invalid. Please purchase a new subscription key.',
    'type' => 'info',
    'success' => false,
    'error' => true,
    'downloadfailmessage' => '',
    )
  • The downloadurl value returned obtained from the download server in the XML may now include placeholders. The above test extensions demonstrate this. Example XML link.
  • The event listeners provide additional flexibility for developers who wish to make use of them. These above tests do not make use of any of these event handlers.

Here is an example of a generic installer plugin which provides similar functionality for update servers which don't provide a JSON response. It also attempts to detect an update site which returns a HTML document and not a ZIP!
plg_installer_generic.zip

@BrainforgeUK BrainforgeUK changed the title 6.1 dev 6.1 dev Improvements to extension updater Sep 9, 2025
@BrainforgeUK BrainforgeUK changed the title 6.1 dev Improvements to extension updater [6.1] Improvements to extension updater Sep 9, 2025
@BrainforgeUK
Copy link
Contributor Author

Why is PHPstan failing?

See lines 91 and 131 in InstallerHelper.php
Refer to:
https://api.joomla.org/cms-5/deprecated.html#repos/joomla-cms/libraries/src/Application/EventAware.php

@richard67
Copy link
Member

@BrainforgeUK PHPstan fails because you added another call to the deprecated getDispatcher, so the count for the exclusion in the phpstan-baseline.neon file does not match anymore.

You can fix that by running ./libraries/vendor/bin/phpstan -b on your branch (after having run composer installso that phpstan is available). This will re-create the phpstan-baseline.neon file. Then commit and push the changed file.

@richard67
Copy link
Member

P.S.: If you don't have a development environment so you can't do what I recommended in my previous comment, then let me know here and I will do it later.

@BrainforgeUK
Copy link
Contributor Author

P.S.: If you don't have a development environment so you can't do what I recommended in my previous comment, then let me know here and I will do it later.

** Yes please, can you do that for me. Thanks.

@BrainforgeUK
Copy link
Contributor Author

P.S.: If you don't have a development environment so you can't do what I recommended in my previous comment, then let me know here and I will do it later.

** Yes please, can you do that for me. Thanks.

!! Help have the same issue with #46413 !!

@richard67
Copy link
Member

@BrainforgeUK I have checked the PHPstan error message. You can do that, too, by checking the changed files on GitHub.

Maybe you should not use Joomla 3 code in your PRs?

@HLeithner Could you maybe advise here what can be done instead of using the deprecated interface "Joomla\CMS\Application\EventAwareInterface"?

@Fedik
Copy link
Member

Fedik commented Nov 7, 2025

Could you maybe advise here what can be done instead of using the deprecated interface

I am not Harald, but it as i already wrote. Ignore it.
The Application implements 2 interfaces which shares getDispatcher() method:

DispatcherAwareInterface::getDispatcher()

and

EventAwareInterface::getDispatcher()

The EventAwareInterface is deprecated and will be removed, however DispatcherAwareInterface will stay and the getDispatcher() method also will stay.

I think PHPStan cannot distinguish this race condition 😉

@richard67
Copy link
Member

I see. Will update phpstan baseline tomorrow.

@richard67
Copy link
Member

I've updated the phpstan-baseline.neon file to make PHPstan pass. Basically it is a false alarm by PHPstan, see Fedir's comment above .

@Fedik
Copy link
Member

Fedik commented Nov 9, 2025

Changes in downloadPackage():

I think changes in downloadPackage() method is unnecessary and over complicated.

As I understand the aim is to show to User the error.
This can be done near:

if (200 != $response->getStatusCode()) {
Log::add(Text::sprintf('JLIB_INSTALLER_ERROR_DOWNLOAD_SERVER_CONNECT', $response->getStatusCode()), Log::WARNING, 'jerror');
return false;
}

Here we can show server message when it return 403, kind of:

if (200 != $response->getStatusCode()) {
   Log::add(Text::sprintf('JLIB_INSTALLER_ERROR_DOWNLOAD_SERVER_CONNECT', $response->getStatusCode()), Log::WARNING, 'jerror');

  if (403 === $response->getStatusCode()) {
    $body = (string) $response->getBody();
    // Show response message, but ignore default server error pages
    if ($body && !str_contains($body, '</body>')) {
        //Translate JLIB_INSTALLER_ERROR_DOWNLOAD_SERVER_MESSAGE="Error downloading the packge: %1$s. %2$s"
        Log::add(Text::sprintf('JLIB_INSTALLER_ERROR_DOWNLOAD_SERVER_MESSAGE', $url, $body), Log::WARNING, 'jerror');
    }
  }

  return false;
}

Nothing special, if the key is wrong Developer just throw 403 on their server and show any text they want to show.
In theory should work simpler than magic JSON.
(We also can check for some response header kind of X-JOOMLA-PACKAGE-MESSAGE, but that can be another "magic" for developers.)

AfterPackageDownloadFailedEvent

This event does not really make sense on first look. If it also just to show message, then probably can be also unnecessary after changes in downloadPackage() method

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Nov 11, 2025
Add logging for 403 response status with custom message.
@BrainforgeUK
Copy link
Contributor Author

Added 403 handler as suggested.

Kept JSON response handling and onInstallerAfterPackageDownload event.
Looking at my update server I prefer that flexibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants